Skip to content

fix(service-analytics): compareTo 带上 measure 自己的 filter,__compare 列不再是另一个 measure (#4820) - #4870

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-4820-compare-measure-filters
Aug 3, 2026
Merged

fix(service-analytics): compareTo 带上 measure 自己的 filter,__compare 列不再是另一个 measure (#4820)#4870
os-zhuang merged 2 commits into
mainfrom
claude/issue-4820-compare-measure-filters

Conversation

@claude

@claude claude Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Fixes #4820

问题

带 filter 的 measure,在当前期是靠一条补充分组子查询(combineFilters(baseFilter, compiled.measureFilters[m]))来限定范围的;但 compareTo 那条路径只发一条涵盖全部 base measure 的位移查询,where 里只有 baseFilter —— compiled.measureFilters 在那条路径上从未被查阅过。

于是同一个 measure,当前期这一列被限定了,对比列没有:

# measures where 窗口
1 revenue — 当前期
2 won_count {"stage":"closed_won"} 当前期
3 revenue, won_count 无 位移

won_count__compare 数的是上个窗口的全部商机,被这个 measure 本来要排除的行精确地放大了。误差只往一个方向走:对比期永远显得更好,所以「本月赢单 vs 上月」的卡片会在什么都没变坏的时候读作暴跌。只有带 filter 的 measure 错,旁边不带 filter 的照常正确 —— 这正是它能一直活着的原因。

修法

没有在 compare 路径上再写一遍「怎么应用 measure filter」。两条路径现在走同一个 runMeasurePass:不带 filter 的 measure 一条查询,带 filter 的每个一条子查询,按维度键合并;splitMeasuresByFilter 是「这个 measure 有没有自己的 filter」的唯一答案。两个窗口之间唯一的差别,就是位移过的 dateRange。

长出第二套规则正是这类缺陷复发的地方 —— 它们下一次改动就会再次分叉。所以抽的是公共函数,不是补丁。

数据集 filter、presentation 的 runtimeFilter、measure 自己的 filter,在两个窗口里以完全相同的方式组合。

代价:设了 compareTo 时,每个带 filter 的 measure 多一条查询。measure 都不带 filter 的 selection 完全不受影响,仍然只发一条位移查询(测试钉住了这一条)。

与 #4708 的接缝:compare 路径现在也会扇出,所以它的补充子查询同样可能整组缺失。fillEmptyGroups 已经覆盖 __compare 列,上个窗口被 filter 清空的分组读作 0(count/sum)而不是空白 —— 和当前期的行为一致,已加测试。

测试

测试的假 service 不是按查询形状返回硬编码数字,而是一个小的内存库:它真的对种子行求值 where 和 dateRange,真的分组。所以丢掉 measure filter,数字会变 —— 这是本 PR 唯一值得钉的东西,查询形状证明不了用户看到的数是对的。

种子刻意让 revenue 在两个窗口都是 300(3×100 = 100 + 4×50),于是能让 won_count__compare 动的只剩「filter 有没有到位移查询」。

改动前后跑同一组用例:

用例 修复前 修复后
位移查询条数 1 2
位移的 won_count 查询 where 不含 closed_won { stage: 'closed_won' }
won_count__compare 5 1
方向(本月 3 赢单) 3 vs 5 = 暴跌 3 vs 1 = 增长

修复前的实际报错(把 fix 临时回退成原来那条扁平查询跑出来的):

AssertionError: expected [ { cube: 'pipeline', …(4) } ] to have a length of 2 but got 1
AssertionError: expected 5 to be 1 // Object.is equality
AssertionError: expected 3 to be greater than 5
AssertionError: expected '{"$and":[{"is_deleted":{"$ne":true}},…' to contain '"stage":"closed_won"'

命令与结果:

npx vitest run --maxWorkers=2                  # 38 files / 511 tests passed
npx tsc --noEmit -p .../service-analytics      # 7 errors,与 main 基线逐条相同(全在我未触碰的既有测试文件里)
                                               # 新增/改动的两个文件 0 error
npx eslint (两个改动文件)                        # 无输出

changeset 等级:patch

splitMeasuresByFilter 没有加进 src/index.ts,包的公开入口面一字未变,可观察到的变化就是 __compare 列的数字被改对了。作为对照,#4708 的 PR #4822 是 minor,因为它确实新增了公开面(fillEmptyGroups 进了 index、emptyGroupValueFor 进了 spec);本 PR 没有对应的东西。

范围

packages/spec/** 零改动;未触碰 packages/lint、skills/**、content/docs/**、content/docs/releases/。

⚠️ #4821(mergeByDimensions 用无分隔符拼接维度键)就在本 PR 改的同一个文件里,本 PR 刻意不碰,留给它自己那一单。

🤖 Generated with Claude Code

https://claude.ai/code/session_015Br2xsJsczFsTR9bvbh2Ny

Generated by Claude Code

@claude
fix(service-analytics): compareTo applies measure-scoped filters, so …
1d4a2aa 
…<measure>__compare is the same measure as its neighbour (#4820)

A measure declared with its own `filter` was scoped by a supplementary grouped
sub-query in the current period, but the `compareTo` pass issued ONE shifted
query over every base measure with only the base filter — `measureFilters` was
never read on that path. `won_count` counted won deals while `won_count__compare`
counted every deal, side by side under one label, biased so the comparison
window always looks better.

Both windows now run the same `runMeasurePass`: unfiltered measures in one
query plus one sub-query per filter-scoped measure, merged by dimension key.
The split lives in one exported `splitMeasuresByFilter` so the two paths cannot
re-diverge; the only difference between them is the shifted dateRange.

Tests build a fake service that really evaluates `where` and groups seed rows,
so the pinned number changes with the fix (won_count__compare 5 -> 1), not just
the query shape. Selections whose measures carry no filter still compare in a
single shifted query.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Br2xsJsczFsTR9bvbh2Ny
@vercel

vercel Bot commented Aug 3, 2026 •
edited
Loading

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 3, 2026 11:34am

Request Review

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling size/m labels Aug 3, 2026
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/service-analytics.

8 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/api/data-api.mdx (via @objectstack/service-analytics)
  • content/docs/api/index.mdx (via @objectstack/service-analytics)
  • content/docs/kernel/services-checklist.mdx (via @objectstack/service-analytics)
  • content/docs/permissions/sharing-rules.mdx (via @objectstack/service-analytics)
  • content/docs/plugins/packages.mdx (via @objectstack/service-analytics)
  • content/docs/releases/implementation-status.mdx (via @objectstack/service-analytics)
  • content/docs/releases/v17.mdx (via @objectstack/service-analytics)
  • content/docs/releases/v9.mdx (via @objectstack/service-analytics)

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

@claude
fix(service-analytics): key the compare-filter test fake without a ra…
0a3e4ec 
…w NUL byte

`check:nul-bytes` (run inside the ESLint job) rejected a literal 0x00 written
into the fake service's group key. A raw NUL makes grep/ripgrep treat the whole
file as binary and return zero matches, so the file drops out of code search
and every grep-based lint; git only scans the first 8000 bytes, so at offset
5440 it still diffed as text.

Keyed with `JSON.stringify(...)` joined by `|` instead, which needs no exotic
byte at all and is unambiguous for the multi-dimension case. Deliberately NOT
the production `mergeByDimensions` key, whose delimiter-free concatenation is
#4821 — the fake must not import a second defect into a test for this one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Br2xsJsczFsTR9bvbh2Ny
@github-actions github-actions Bot added size/l and removed size/m labels Aug 3, 2026
@os-zhuang
os-zhuang marked this pull request as ready for review August 3, 2026 11:39
@os-zhuang
os-zhuang added this pull request to the merge queue Aug 3, 2026
@os-zhuang Claude

os-zhuang commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

复核通过 —— ACCEPT,已标 ready 并送合并队列

1. 没有长出第二套规则 —— 这是本单最重要的要求

我派发时写的是:「如果两条路径各自实现一遍『怎么应用 measure filter』,下一个缺陷就在它们再次分叉的地方。」

实际做法是抽出 runMeasurePass(不带 filter 的 measure 一条查询 + 每个带 filter 的 measure 一条补充子查询,按维度键合并)与 splitMeasuresByFilter,executeSelection 与 runCompare 都调用它。两条路径从此共用一份语义,不会再次分叉。

2. 假 service 真的在求值 —— 这是「数值可区分」要求的正确做法

假 service 不是按查询形状返回硬编码值,而是真的对种子行求值 where 与 dateRange 并分组,所以丢掉 measure filter 数字会变。

而且种子设计得很巧:让 revenue 在两个窗口都等于 300(3×100 = 100+4×50),唯一能动 won_count__compare 的就只剩 filter 是否到位。这是把变量隔离到只剩被测那一个。

反向验证给出的是用户真正看到的东西:

won_count__compare 修复前 5、修复后 1,方向从「3 vs 5 暴跌」变回「3 vs 1 增长」。

一个「本月赢单 vs 上月」的卡片,修复前显示的是暴跌,实际是增长。断言查询形状证明不了这个,断言数字才行。

无 filter 的 measure 用例修复前后都通过(仍只发 2 条查询),主路径未误伤。

3. 与 #4708 的接缝处理对了

基于 PR #4822 合并后的 main 工作,fillEmptyGroups 的位置未动,现在自然覆盖 compare 路径新长出的扇出接缝 —— 上个窗口被 measure filter 清空的分组读 0 而非空白,并补了测试。两条相邻的 bug 没有互相拆台。

4. NUL 字节

首轮 CI 的 ESLint 红是 check:nul-bytes 抓到测试里一个裸 0x00(不是 lint 规则)。已改键法并作为新提交推送(未 force-push),第二轮 20 checks 全绿。

顺带记一个今天学到的东西:裸 NUL 会让 git 把文件判为二进制,于是 GitHub 上这个测试文件的 diff 显示成 +0/-0 —— 看起来像个空文件。我一开始就是这么误判的,实际是 347 行。

5. changeset patch 正确

splitMeasuresByFilter 刻意不加进 src/index.ts,包的公开入口面未变。与本仓「无新增包根导出面 ⇒ patch」一致(#4822 取 minor 是因为它确实新增了公开行为面,两者不同)。

packages/spec/** 零改动 ✅。

⚠️ 给 #4821 的一条警告 —— 它推翻了那个 issue 里「显而易见」的修法

dev 按指示没碰 #4821,但报告里留了一条会改变其修法的发现,我原样转到 #4821 并摘录于此:

issue 建议的 ${d}=${JSON.stringify(row[d] ?? null)} 键法会顺带改掉类型强制语义 —— 现行 String(row[d]) 让数字 1 与字符串 '1' 合并成同一组,而 JSON.stringify 会把它们分开。dataset-executor.ts 自己的 compareValues 注释就写明「some drivers return SUM results as numeric strings」,主查询与补充子查询若对同一维度返回不同 JS 类型,换成 JSON.stringify 会让原本能合并的行不再合并 —— 一个静默的新缺陷。

也就是说:#4821 那个看起来最自然的修法,会用一个新的静默缺陷换掉旧的。dev 给的替代方向是保留 String() 的逐段强制、只把拼接改成无歧义形式(如长度前缀 s.length + ':' + s),null 与 '' 的区分另用哨兵值。

同时 #4821 的优先级被本 PR 抬高了:compare 路径现在也会扇出并逐 measure 合并,所以一次键碰撞还会污染 __compare 列,受影响面比该 issue 正文估计的更大。

一条账本漂移(仅记录)

scripts/check-type-check-coverage.mjs 登记 service-analytics 的 errors: 3,实测 7。不是回归也不是门禁失败(该包无 typecheck 脚本,条目是冻结的 DEBT 计数,check:type-check-coverage 仍 OK),但那个数字已与实际脱节。与 #4855(qa/dogfood 的 tsconfig 无人跑)同属「DEBT 账本本身在腐烂」这一类。

Generated by Claude Code

@os-zhuang os-zhuang mentioned this pull request Aug 3, 2026
Closed
Merged via the queue into main with commit 2f05139 Aug 3, 2026
21 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-4820-compare-measure-filters branch August 3, 2026 11:51
This was referenced Aug 3, 2026
Merged
Open
Closed
Merged
@xuyushun441-sys xuyushun441-sys mentioned this pull request Aug 3, 2026
Merged
@github-actions github-actions Bot mentioned this pull request Aug 3, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

compareTo drops every measure-scoped filter, so <measure>__compare silently reports a different measure than the column beside it

2 participants

@os-zhuang @claude